feat: create metadata entries generator #272
Conversation
|
Blocking this until #275 is merged. |
bin/commands/generate.mjs
Outdated
| const releases = await getAllMajors(); | ||
|
|
||
| await Promise.all( | ||
| docs.map(async doc => { | ||
| const { runGenerators } = createGenerator(doc); | ||
|
|
||
| await runGenerators({ | ||
| generators: opts.target, | ||
| input: opts.input, | ||
| output: opts.output && resolve(opts.output), | ||
| version: coerce(opts.version), | ||
| releases, | ||
| gitRef: opts.gitRef, | ||
| threads: parseInt(opts.threads, 10), | ||
| }); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
I know this PR is still a draft, but I'm concerned that this will overload the system's CPU. We already run each generator on it's own thread, why do we need to iterate on each doc as well?
There was a problem hiding this comment.
@avivkeller It was a misunderstanding, fixed now. Can you take a look?
araujogui
force-pushed
the
refactor-parsers
branch
from
f6ba0c8 to
37a572d
Compare
|
@araujogui I saw there is a blocked label, could you elaborate what's blocked? |
The current linter relies on metadata entries, which are no longer easily available because it's now a generator. I’ve blocked this until #275 is merged, as it removes the linter's dependency on metadata. |
araujogui
force-pushed
the
refactor-parsers
branch
from
37a572d to
2f6cf99
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #272 +/- ##
=======================================
Coverage 91.88% 91.88%
=======================================
Files 56 56
Lines 4142 4142
Branches 174 174
=======================================
Hits 3806 3806
Misses 334 334
Partials 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@nodejs/web-infra this is ready for reviews |
| import createQueries from '../../../utils/queries/index.mjs'; | ||
| import { getRemark } from '../../../utils/remark.mjs'; | ||
| import { createNodeSlugger } from '../../../utils/slugger/index.mjs'; | ||
| import createMetadata from '../../../metadata.mjs'; |
There was a problem hiding this comment.
nit: are any of these only in this generator? If so, can we move them here?
There was a problem hiding this comment.
I moved the slugger file, but metadata.mjs types is also used on src/utils/queries/index.mjs
| @@ -109,7 +109,7 @@ const createMetadata = slugger => { | |||
| * The Navigation entries has a dedicated separate method for retrieval | |||
| * as it can be manipulated outside of the scope of the generation of the content | |||
| * | |||
| * @param {import('vfile').VFile} apiDoc The API doc file being parsed | |||
| * @param {{stem?: string, basename?: string}} apiDoc The API doc file being parsed | |||
There was a problem hiding this comment.
Yeah, it is but some properties like stem and basename are not enumerable in VFile, so I needed to serialize them before sending to worker threads.
There was a problem hiding this comment.
I can create a PR on the VFile repo to mark these as enumerable, but I'm not sure if there are any drawbacks.
| return { | ||
| file: { | ||
| stem: resolvedApiDoc.stem, | ||
| basename: resolvedApiDoc.basename, | ||
| }, | ||
| tree: apiDocTree, | ||
| }; |
| export interface ParserOutput<T> { | ||
| file: { | ||
| stem?: string; | ||
| basename?: string; | ||
| }; | ||
| tree: T; | ||
| } | ||
|
|
Description
Creates metadata entries generator
Related Issues
Fixes #271
Check List
node --run testand all tests passed.node --run format&node --run lint.